Skip to content

Add useRequests(): queue rows as React hooks, cached per client - #76

Open
dmurphy5 wants to merge 18 commits into
masterfrom
dylan/request-rows-hook
Open

dmurphy5 wants to merge 18 commits into
masterfrom
dylan/request-rows-hook

Conversation

@dmurphy5

@dmurphy5 dmurphy5 commented Nov 10, 2026 •

Copy link
Copy Markdown

Why

The app keeps its own copy of getRequests() to show upload rows, in a hook it wrote. That copy has two gaps. Native sends no event when it forgets a row after the ack, and no event when cancel() removes a settled row. The app patches the second gap by wrapping cancel(). The first gap stays: a completed row can stay on screen after native forgot it.

The library sends the acks and the cancels itself, so it can close both gaps. This PR moves the hook into the library. The app then creates requests and handles outcomes, and keeps no copy of the queue.

This is display only. It does not change what is sent, acknowledged, or cancelled.

API

On each client:

  • useRequests(filter?) returns the rows. It re-renders when a row appears, changes state, or goes. It does not re-render on progress.
  • useRequestsWithProgress(filter?) returns the same rows with the bytes of each progress event. It also re-renders on progress.
  • filter is { key?, id?, includeCancelled? }. Cancelled rows are left out unless includeCancelled is true. The app hides them today, and a cancelled row has no work left.
  • addRequestsListener(listener) returns { remove }. It calls listener with the rows when they change. There is no call when it registers. It is for code outside React, such as a store or a logger.
  • getCachedRequests(filter?) returns the rows the hooks see. While nothing listens, it reads native on each call. A result filtered by key or id is a new array each call, so it is not a hook snapshot.
  • Completed rows are included until their outcome is acknowledged. The README says not to treat them as work still to do.

On each definition, define() now also returns useRequests(filter?) and useRequestsWithProgress(filter?). They return that key's rows with vars typed as the definition's vars. The filter has no key.

Results keep the same array while the rows they hold are the same, also when the filter is a new object each render. A hook filtered by id looks its row up in an index shared by all hooks, and re-renders only when that row changes. Hooks without an id share one filtered array per key.

The README tells apps to take the hooks out first (const { useRequests } = uploads). The hooks lint rule does not check uploads.useRequests(), because it checks X.useY() only when X starts with a capital letter.

How the cache works

  • The client reads native when the first hook or listener subscribes, also when a render read it just before. Native emits from its own thread, so an event can reach no listener between the two.
  • Each state event replaces the row it carries. Other rows keep their objects.
  • When the ack of a completed or cancelled outcome resolves, the row goes. If the row has moved on, for example a cancelled entry that a same-id mutate() reopened, the client asks native for that row. src/delivery.ts calls a new onAcked after a successful ack. A throw there is a warning only.
  • When cancel(id) resolves on a settled row, the row goes. A live row waits for its cancelled event and its ack.
  • An error row stays until cancel() or a same-id mutate(). Native sends a state event for a same-id mutate(), so that needs nothing extra. I checked this in the iOS and Android code.
  • iOS can settle a live row during an ack with no event. After the ack of an error outcome, the client reads that row from native if it does not show it as an error.
  • A state row with reason: 'unhandled-key' is built from the outcome, not the entry. The client reads that row from native instead.
  • Native progress is listened to only while a progress hook is mounted.
  • The cache's state and progress handlers catch their own errors and send them to onWarning, so they cannot stop the app's other listeners.
  • With nothing listening, an ack or a cancel() drops the cached rows instead of patching them. No native read runs in the delivery lane while the journal drains at launch.
  • When the last listener leaves, the rows stay. The next subscribe checks them against native.
  • A failed native read keeps the rows and sends a warning to onWarning.

A late state event no longer brings back a finished row. Android emits a run's outcome before its last state event, so the ack could remove the row and the late event then added it back. The cache now remembers the ids it removed because native forgot them, at most 256, and checks a later state event for one of those ids against native. After cancel(), the cache keeps a cancelled row until native forgets it.

What to look at

  • src/requests.ts: the cache, the filter selector, and the hook.
  • src/index.ts: the wiring, the cancel() change, and the define() wrapper.
  • src/delivery.ts: onAcked.
  • src/testing.ts: settle() now resolves one promise turn later, after the client has handled the ack. A test that awaits settle() then sees the row gone. This depends on delivery's ack() handling the ack in the turn after ackEvents() resolves; comments in both places say so. A macrotask would remove that dependency, but it would make settle() wait on timers, which Jest fake timers hold.
  • Defined and UploadClient gained required members. A hand-built test double of either type must add them. The CHANGELOG says so.
  • example/RNBGUExample/jest.config.js: maps react to the example's copy. Without it, src/ and react-test-renderer load two Reacts.

Tests

  • yarn lint:ci: passes.
  • yarn typecheck: passes, with new type tests in src/__typetests__/requests.ts.
  • yarn test: 282 tests pass. 67 are new: 62 in src/__tests__/requests.test.ts and 5 in delivery.test.ts. They include an event lost between a render read and the subscribe, a listener that calls cancel() and getRequests() while a row drops, and a count that shows an id lookup does not walk the rows.
  • Example app Jest: 13 tests pass. 9 are new in __tests__/useRequests.test.tsx. They render the hooks with react-test-renderer over the fake native, including under StrictMode and with a filter that changes.
  • Benchmark with 300 rows, 200 id-filtered hooks and 5 unfiltered hooks: each event now reads 600 rows (one index build and one shared filter) instead of about 61,500. A state event takes 0.10 ms instead of 0.80 ms. Render counts are the same.

Merge order

PR #67 (dylan/deliver-late-definitions) also changes src/delivery.ts. Whichever of the two lands second merges master in.

🤖 Generated with Claude Code

dmurphy5 and others added 5 commits November 10, 2026 13:28
A native enqueue that answers after enqueueTimeoutMs must resolve mutate(),
not reject it. Native's own rejection must still reject. The warning fires
one time, and the handler still runs after the caller has the id. These
tests fail on the current code, which rejects at the timeout.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
mutate() rejected when native enqueue had not answered within
enqueueTimeoutMs. Native could still store the entry and upload it, so the
app recorded a failure for a request that then went through. On iOS a large
chunked file can hold the native queue long enough to cause this.

Now mutate() warns one time at enqueueTimeoutMs and keeps waiting. It
settles only with native's answer. Delivery for the id waits on the same
promise, so a handler still never runs before the caller has the id.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
- configure({ onWarning }) receives every library warning instead of
  console.warn, so an app can send a slow-enqueue warning to its error
  tracker. The client builds one warn function that reads the sink at each
  call, so a sink set by configure() is used by later warnings. A throw from
  the sink falls back to console.warn.
- createFakeNative().holdNext('enqueue') keeps the next enqueue pending
  until released, so a test can model a slow native.
- Android also runs enqueue on one thread, so the docs no longer say that
  only iOS waits behind earlier enqueues.
- The CHANGELOG marks the mutate() change as a behavior change and says that
  only a rejection from native is native's error.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The client keeps one in-memory copy of getRequests(). It reads native when
the first hook or listener subscribes, and each state event replaces the
row it carries. useRequestsWithProgress() overlays the bytes of progress
events. Filtered results keep their array while their rows are the same.

Native forgets a row with no event in two cases that this library's JS
starts, so the cache drops the row itself: when the ack of a completed or
cancelled outcome resolves (delivery's new onAcked), and when cancel()
resolves on a settled entry. An error row stays until cancel() or a
same-id mutate(), which emits a state event.

define() now also returns useRequests() and useRequestsWithProgress() for
the definition's key, with typed vars. The fake's settle() resolves after
the client has handled the ack, so the hooks are up to date then.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dmurphy5
dmurphy5 added this pull request to stack #77 November 10, 2026 19:37
dmurphy5 and others added 6 commits November 10, 2026 14:50
Correctness:
- The first subscribe always checks native. Native emits from its own
  thread, so an event can go to no listener between a render's read and
  the subscribe. reconcile() keeps the same array when nothing changed.
- The ack of an error outcome reads that row from native when the cache
  does not show it as an error: iOS can settle a live row during an ack
  with no event.
- An unhandled-key state row is read from native, because JS builds it from
  the journaled outcome and a same-id mutate() may have reopened the entry.
- addRequestsListener() is not called while it registers, and not called
  when its rows did not change (only a hidden cancelled row did).

Performance:
- An id index per rows array, shared by all selectors. An id-filtered hook
  looks its row up instead of walking every row.
- Hooks without an id share one selector per source, key and
  includeCancelled.
- Row equality compares scalar fields first and allocates nothing. vars
  is always compared.
- getCachedRequests() with nothing listening takes native's rows as they
  are; the next subscribe reconciles them.
- Native progress is listened to only while a progress hook is mounted, and
  a dropped row's progress is forgotten.

Docs: take the hooks out of the client before use, because the hooks lint
rule skips lowercase X.useY(); completed rows stay until their ack.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The cache's state and progress handlers run inside native's event
  callbacks. A throw there now goes to the warning sink instead of
  escaping and stopping the app's later listeners.
- With nothing listening, an ack or a cancel() drops the cached rows
  instead of patching rows that may be out of date. No native read runs in
  the delivery lane while the journal drains at launch; the next subscribe
  or read reads native.
- Comments in delivery's ack() and the fake's ackEvents() name the one
  promise turn that settle() counts on.
- CHANGELOG: Defined and UploadClient gained required members, which
  breaks hand-built test doubles of those types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…re() first

- createFakeNative(): while an enqueue is held, later enqueue calls wait
  behind it and run in order on release, as native runs enqueue one call at
  a time on both platforms. reset() drops the waiting calls too.
- configure() checks every option before it stores any, so an invalid
  option no longer leaves the configuration half-applied.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Native warnings go to takeDiagnostics(), never to onWarning. The README,
the type doc and the CHANGELOG now say so.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@dmurphy5
dmurphy5 marked this pull request as ready for review November 10, 2026 20:52
@elliottkember
elliottkember requested a review from a team November 12, 2026 17:13
dmurphy5 and others added 4 commits November 12, 2026 15:13
Brings in the retry.transient rules (#62-#64).

Conflicts:
- CHANGELOG.md: kept master's retry.transient paragraph, then this branch's three paragraphs.
- src/index.ts configure(): kept this branch's onWarning check after the maxVarsBytes check, so every check (retry first, from master) still runs before any setting is assigned. Dropped this branch's comment "Every option is checked before any is stored..." because master's comment at the top of configure() says the same.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Brings in master's retry.transient rules (#62-#64) through dylan/mutate-waits.

Conflict:
- CHANGELOG.md: kept master's retry.transient paragraph first, then this branch's two paragraphs, then the dylan/mutate-waits paragraphs that already followed them.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
On Android a run emits its outcome before its last state event. When the
ack of the outcome forgot the entry before that state event reached JS,
the cache removed the row at the ack, then the late `completed` event added
it back, and it stayed until the next launch.

The cache now keeps the ids it removed because native forgot them (up to
256, oldest out first). A state event for one of these ids is checked
against native: the row comes back only when native holds it, for example
after a same-id mutate(). An id leaves the set when a native read shows a
row for it. The first event of a new id is applied as before, with no read.

The same applies to an ack while nothing listens, and to cancel(): a row
still shown live at cancel() is now read from native, because native may
have settled it already and the cancel forgot it.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…vents

Follow-up to the late state event fix, after review. That commit's message
said the stale row stayed until the next launch. It stayed until the next
first subscribe or that id's next mutate().

- cancel() on a row shown as 'cancelled' now asks native. On Android the
  cancelled state event arrives before cancel() resolves, and native keeps
  the row until the ack. The cache removed it at once before, so with
  includeCancelled the row went early, and stayed hidden when the ack
  failed.
- A settled state event for an id the cache does not hold now asks native.
  A new life starts live, so this costs a read only for a late event. It
  covers an id pushed out of the 256-id set.
- When that native read fails, a live event, or an event for a row already
  shown, is applied. A reused id is then not hidden for its whole new life.
- Tests: Android's event order through the client and the fake, a failed
  ack of a cancel, the set overflow, failed reads, and cancel() of an id the
  cache did not hold. Two cancel tests now check the ack, not cancel(), as
  what removes the row.
- README and CHANGELOG wording.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
dmurphy5 and others added 2 commits November 13, 2026 14:48
Brings in #65, #74, #78 and #80.

CHANGELOG.md: kept every entry; master's entries first, then this branch's
three (mutate() waits, onWarning, holdNext).
src/__typetests__/define.ts: kept both blocks (this branch's onWarning
checks and master's takeDiagnostics() checks).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Brings in the updated base, which now has #65, #74, #78 and #80 from
master.

CHANGELOG.md: kept every entry; master's entries first, then #71's, then
this branch's (row hooks, fake settle() timing).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Base automatically changed from dylan/mutate-waits to master November 13, 2026 20:02
#71 is now in master as a squash, with the same tree as the #71 tip this
branch already merged. So master adds nothing here; the conflicts come
from the squash dropping the shared history.

CHANGELOG.md, src/delivery.ts, src/index.ts: kept this branch's side, which
is #71's text plus the row cache changes. The diff against master is the
same as this branch's own diff before the merge.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant